🦋 Changeset detected

Latest commit: 1edd66e

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Walkthrough

Adds per-variable secret support across sync/import flows. New changeset added. API schema and client types gain optional secrets, parentSecrets, and parentVariables fields. The webapp import route reads body.secrets / body.parentSecrets and sets isSecret on imported variables; EnvironmentVariablesRepository.create signature now accepts isSecret. The build extension syncEnvVars accepts isSecret on items and tracks secrets and parentEnvSecrets alongside env / parentEnv. The CLI exposes and forwards secrets and parentSecrets. Some template/string formatting tweaks were also applied.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Late to the game, but would love to see this as well. Curious if the original work is still good enough or if this needs a new attempt with so much changing in the last year-or-so

The per-variable isSecret flag added in api.v1.projects.$projectRef.envvars.$slug.import.ts never reaches storage — this PR is currently a no-op on the import endpoint.

The problem

The route attaches isSecret to each item in the variables array:

variables: Object.entries(body.variables).map(([key, value]) => ({
  key,
  value,
  isSecret: body.secrets?.[key] ?? false,
})),

But EnvironmentVariablesRepository.create() only reads options.isSecret (a single top-level flag applying to all variables in the call). It never reads variable.isSecret from the array items. The CreateEnvironmentVariables schema in apps/webapp/app/v3/environmentVariables/repository.ts confirms this — variables is typed as { key, value }[] with no per-item isSecret. The extra field passes TypeScript via structural assignability and is silently discarded at runtime.

Net effect for an existing variable that was previously a secret:

• The update branch (lines 214–231) only writes isSecret when options.isSecret !== undefined — so the existing value is preserved regardless of what's in body.secrets.
• New variables get isSecret: undefined (or whatever is inherited from the parent env), again ignoring body.secrets.

So passing isSecret: true does not promote a var to a secret, and passing isSecret: false does not un-secret one. The wiring stops at the route boundary.

Suggested fix

Move isSecret onto the per-variable item all the way down:

1. Extend the CreateEnvironmentVariables schema so variables items carry an optional isSecret: z.boolean().optional().

2. Inside create()'s per-variable loop, compute const perItemIsSecret = variable.isSecret ?? options.isSecret; and use that in place of options.isSecret for the inheritance lookup, the canSkip check, the update's conditional isSecret write, and effectiveIsSecret on insert.

3. In the import route, drop the ?? false fallback:

   isSecret: body.secrets?.[key],

   Keeping it as undefined when the key is missing from secrets preserves the existing tri-state semantics: undefined = leave existing value / inherit from parent, true = mark as secret, false = explicitly un-secret. With ?? false, every import without a secrets map would un-secret every previously-secret variable.

Why per-item rather than two calls bucketed by isSecret

A split-call approach (one call with options.isSecret: true, another with options.isSecret: false) breaks the parent-env inheritance at lines 162–177 of the repository — both calls would pass an explicit boolean, so options.isSecret === undefined is never true and the parentEnvironmentId lookup never runs. The route deliberately passes parentEnvironmentId for exactly that fallback. Per-item preserves undefined for variables not named in body.secrets, so inheritance still works for them.

It's also a smaller change — create() already iterates one variable at a time, so it's just swapping which field is read inside the existing loop instead of adding a second transaction and another project/variable lookup pass.